Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix matchstick prediction issues #31418

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

beck-thompson
Copy link
Contributor

@beck-thompson beck-thompson commented Aug 25, 2024

About the PR

Matchstick was always miss predicting getting inserted into a container. There was an outdated method that I fixed as well.

Technical details

The only sussy thing I had to do was always make the event handled if a matchstick is used on a matchbox. If you don't do that and check for it being lit and then making it list if its not, it starts misrepresenting on the second round because the match becomes lit and the event wouldn't be handled.

image
* This works the first time but the second time it will never trigger so it won't get handled and gets inserted.

Media

2024-08-24.22-39-04.mp4

Requirements

  • I have read and I am following the Pull Request Guidelines. I understand that not doing so may get my pr closed at maintainer’s discretion
  • I have added screenshots/videos to this PR showcasing its changes ingame, or this PR does not require an ingame showcase

@Winkarst-cpu
Copy link
Contributor

Oh, I think you need to update yml of matches too...

@beck-thompson
Copy link
Contributor Author

Your right but I have no idea why its failing... Isn't the default variable name lowercase? It was:
[DataField("duration")] but the name was still just Duration which should default to the same thing?

@Winkarst-cpu
Copy link
Contributor

Try removing read-only access, I don't know.

@Winkarst-cpu
Copy link
Contributor

Uh... The old one was read-only too...

@beck-thompson
Copy link
Contributor Author

OK we couldn't figure out why it always fails when the datafield names are removed (Even when the names would still work...). Just leaving them there!

Content.Server/Light/EntitySystems/MatchstickSystem.cs Outdated Show resolved Hide resolved
Content.Server/Light/EntitySystems/MatchstickSystem.cs Outdated Show resolved Hide resolved
Comment on lines 2 to 8

// TODO make changes in icons when different threshold reached
// e.g. different icons for 10% 50% 100%
[RegisterComponent]
public sealed partial class MatchboxComponent : Component
{
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs networking.

Comment on lines 24 to 30

/// <summary>
/// The time that the match will burn out. If null, that means the match is unlit.
/// </summary>
[AutoNetworkedField]
public TimeSpan? TimeMatchWillBurnOut = null;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs autopausedfield.

Content.Shared/Lightning/EntitySystems/MatchboxSystem.cs Outdated Show resolved Hide resolved
@metalgearsloth metalgearsloth added the Status: Awaiting Changes This PR needs its reviews addressed or changes to be made in order to be merged. label Sep 20, 2024
@github-actions github-actions bot added Status: Needs Review This PR requires new reviews before it can be merged. and removed Status: Awaiting Changes This PR needs its reviews addressed or changes to be made in order to be merged. labels Sep 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Needs Review This PR requires new reviews before it can be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants